⚠️ No Changeset found

Latest commit: 942228e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Potential fix for #16594

Playground

pnpm add https://pkg.pr.new/svelte@16595

Playground demonstrating fix: https://svelte.dev/playground/719a2a834fd34d02a204ac4b7c65973b?version=pr-16595

@Rich-Harris is there any chance you could have a look at this? It's a small but important detail in the reactivity system.

Solution looks sound from a first look, though can you make it so that the effects property is turned into a special value (the UNITIALIZED symbol for example) and check for that instead of using another flag (so that we have more room for other potential flags in the future)?
Also this needs a test (add one, e.g. your minimum reproduction, to runtime-runes)

@dummdidumm thanks, I'll have a look at using UNINITIALIZED. Did you want me to add another test, or is the one I added to the end of packages/svelte/tests/signals/test.ts sufficient?

I'm a bit out of depth here, but based on your suggestion this is the best I could come up with for setting the effects to a special value: gitbutlerapp@75d3381

Having the effects be of type null | effect[] | typeof UNINITIALIZED does not sit too well with me, since it affects places where the value is read. What would you say the path forward is?

There's a subtle question around expected behaviour here: is it right that effects are destroyed when nothing is listening to a derived?

Consider this example:

<script>
	class Timer {
		constructor(text) {
			this.elapsed = $state(0);
			this.text = $derived(text + ': ' + this.elapsed);

			$effect(() => {
				const interval = setInterval(() => {
					this.elapsed += 1;
				}, 1000);

				return () => clearInterval(interval);
			});
		}
	}

	let a = $state('hello');
	let b = $derived(new Timer(a));

	let visible = $state(true);
</script>

<input bind:value={a} />
<button onclick={() => visible = !visible}>toggle</button>

{#if visible}
	<p>{b.text}</p>
{/if}

If visible becomes false after a few seconds, the effect is destroyed (this.elapsed stops incrementing) and this.text will be stuck at hello: 3 or whatever when visible becomes true again. That's clearly wrong.

With this PR, the effect is recreated when visible becomes true again. That's good. But it means you recreate the derived (and restart the counter) whenever visible goes from true to false to true even though the value of a hasn't changed. It's like if a tree fell in a forest and no-one was around to hear it, but then someone came to the forest and suddenly the tree was standing again.

To me it feels like the value of b should entirely depend on the value of a, not on a and whether or not something happens to be reading it. Of course if we were serious about that, we would need to execute deriveds when they were first created (which would solve other problems — #16483) but we can get slightly closer with this diff:

diff --git a/packages/svelte/src/internal/client/runtime.js b/packages/svelte/src/internal/client/runtime.js
index 22a1890e0f..363946f292 100644
--- a/packages/svelte/src/internal/client/runtime.js
+++ b/packages/svelte/src/internal/client/runtime.js
@@ -403,6 +403,7 @@ function remove_reaction(signal, dependency) {
        if (
                reactions === null &&
                (dependency.f & DERIVED) !== 0 &&
+               /** @type {Derived} */ (dependency).effects === null &&
                // Destroying a child effect while updating a parent effect can cause a dependency to appear
                // to be unused, when in fact it is used by the currently-updating parent. Checking `new_deps`
                // allows us to skip the expensive work of disconnecting and immediately reconnecting it

A reasonable person could say that as soon as you start using effects in that manner, you're colouring so far outside the lines that you shouldn't have any expectations of the behaviour. I'm not sure. Thoughts?